Stop TES core-data misses from clearing retweet and reply ids - #94
Open
AdemVessell wants to merge 1 commit into
Open
Stop TES core-data misses from clearing retweet and reply ids#94AdemVessell wants to merge 1 commit into
AdemVessell wants to merge 1 commit into
Conversation
Thunder/Phoenix populate retweet and reply ids before TES hydration. A TES miss was applied as PostCandidate::default(), which wiped those ids while leaving author_id set, so the candidate survived hydration filtering and ranking treated retweets as originals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
Thunder and Phoenix retrieval set
retweeted_tweet_id/in_reply_to_tweet_idon the candidate before TES hydration.CoreDataCandidateHydratortreats a TES miss (Ok(None)or a missing map entry) asOk(PostCandidate::default()). CachedHydrator caches that empty payload.update()then assignsNoneover the source-populated ids.author_idwas already merge-safe (if candidate.author_id == 0). The graph fields were not.CoreDataHydrationFilterkeeps the candidate becauseauthor_idis never cleared, so the wiped ids reach ranking.AgeFilterderives age from the snowflake id, not core data, so it does not drop the card either.A TES miss is not "this is not a retweet." Downstream:
vf_candidate_hydrator.rsonly submits the underlying post to VF whenretweeted_tweet_id.is_some(). Wiped, that post is never visibility-filtered — bypassingAuthorBlockViewer,AuthorIsSuspended,AuthorIsDeactivated,ContainNsfwMedia,ReportedTweet,TweetIsBouncedon the original.should_drop_ancillaryskips its retweet check andAncillaryVFFilterkeeps the candidate. The retweet card itself still reaches VF; the original does not.RetweetDeduplicationFilterkeysretweeted_tweet_id.unwrap_or(tweet_id), so duplicate retweets of one source post all survive0instead of the source idFix
Keep the miss as
Ok(default)so it can still be negative-cached.update()no-ops on that empty payload so Thunder/Phoenix ids survive. A real TES hit still assigns, includingNone, so TES remains authoritative when it says the post is not a retweet.is_tes_miss_payloadis a structural stand-in for provenance ("this payload came from a miss"). The hydrator trait isVec<Result<C, String>>; aCacheValueenum would be cleaner but is a framework change. A real TES hit always carriesauthor_id, so the six-way conjunction does not skip a payload that should apply.Tests
Ok(negative cache) andupdate_allleaves idsThe public export does not include a Home Mixer Cargo manifest, so
cargo teststill needs X's build environment.